-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/soft delete #5034
Feat/soft delete #5034
Conversation
…pplying the limit method
Hi, I have forked TypeORM and added some features here: TypeORM+. The README.md may be useful to show you more details about This PR. |
@iWinston thank you for this legendary effort ❤️ I hope this can be pulled in soon. |
@iWinston if you set up a BAT wallet so I can tip you, I'll buy you a coffee. This is really great. |
Waiting for release) |
haha great 😆 |
This will be really helpful! Thanks |
Excited for this! Much needed. 👏 |
I realize this is annoying and not helpful to the discussion, but is there any estimated timeline on merging this? I think there are a lot of us who are trying to figure out whether to write our own implementation / use this fork until it's merged / just wait... Good things take time, but if there's any possibility to merge this it would be greatly appreciated! |
@pleerock let the community review this and give you a information if its ok to merged or not, so itll be less burden to you sir. just a suggestion because I think community are waiting for this. please 🙏 |
Okay since so many people want this feature, let's try to revisit the concept and merge this PR. |
@iWinston good job on implementation, looks really good. Few questions:
|
@pleerock It would be wonderful if we could get this feature merged, it would allow me to remove quite a bit of duplicated code from my builds. I do agree that it would be beneficial (and probably could be required) to support the filtering of soft-deletes in a find query. |
Thank you for your attention. @pleerock
Let me show you how the Query Scopes work: Query ScopesQuery scopes allow you to add constraints to all queries for a given entity. You can register scopes in your entity: 1. Registering Scopesimport { DeleteDateColumn } from 'typeorm-plus'
export class Entity {
static scope = {
'default': {
deletedAt: IsNull()
},
'myScope': {
deletedAt: Not(IsNull())
}
}
} 2. Applying Scopes To QueryBuilderWhen you are calling import {createConnection} from "typeorm-plus";
import {Entity} from "./entity";
createConnection(/*...*/).then(async connection => {
const repository = connection.getRepository(Entity);
await repository.createQueryBuilder().setScope("myScope").getMany();
}).catch(error => console.log(error)); The param 3. Applying Scopes To RepositoryWhen you are calling import {createConnection} from "typeorm-plus";
import {Entity} from "./entity";
createConnection(/*...*/).then(async connection => {
const repository = connection.getRepository(Entity);
// Delete a entity
await repository.find({
scope: 'myScope'
});
}).catch(error => console.log(error)); The property 4. Working with Soft DeleteTypeORM's own soft delete functionality utilizes global scopes to only pull "non-deleted" entities from the database. If the |
"scopes" you are talking about is a different functionality and should come from a separate discussion and PR if we decide to go with it. Can we implement soft deletion without them? I'm sure most people just want either to load records with soft deleted rows included or records without them, most probably by default. Thanks |
@pleerock We can just add a default "non-deleted" condition in this functionality. And maybe we also need to add the But if we adds "scopes" functionality in the future, should we change the |
I don't think so. soft deletion is kinda standalone feature
can you implement that? |
@avin-kavish i think the compelling reason -- and I truly mean no disrespect or ingratitude towards @pleerock, @Kononnable, or any other contributor -- is that typeorm's roadmap / release cycle has had its ups and downs since the beginning; 'soft delete' in particular was first proposed in june 2017, with significant interest from the community (#534), but it never took off. coupled with the fact that this forum is no stranger to cynical discussions about the state of the project overall, i wouldn't be surprised if @iWinston simply wanted to build some features on his own timeline. but as @johannesschobel said, the work is now merged into the main project, and to me, the end result of this thread has been a very positive signal that typeorm is alive and well (and listening to its community) |
@johannesschobel I know this one is merged, but there's more plus features in the other one. Suggestion: Instead of TypeORM+, being a fork, can't we have it so that it will augment this library when imported, or maybe it can have this as a dependency? (I don't know if it's possible though) @iWinston Its a common pattern in C#/ASP.NET to have a library and then another library that extends it's functionality. Since C# supports "extension methods" and that can be attached to classes from other libraries and also class inheritance, it's possible to do this without requiring to fork a project. I imagine the same can be done with |
@avin-kavish Hi, I have tried to patch TypeORM to add extra functionalities : TypeORM-patch, but it's too complex to finish it. Sorry, maybe I shouldn't name it as TypeORM+, I just attend to add more functionalities for our own team at the beginning. As more and more developers noticed the repo, I kept the repo just intended to get advice from them easily. And when the functionalities became mature, I made some PRs for TypeORM. I have mentioned this on 1 Oct 2019. Thanks for @pleerock and @Kononnable, TypeORM is a wonderful repo and very helpful for me. |
Thank you to point it out! I missed this part, maybe you can make a PR for it. |
Hi, I can make a PR for the doc if it's necessary. |
Adding an extensibility framework to TypeORM would be really great. That way we can start bringing new features at our own pace without resorting to forks. Thanks again for all your work @iWinston |
@iWinston Hello sir. for sure I can create a PR for this. Can I use your repo though so you can guide me? this is embarrassing but I'm not really sure about what's happening in the code. :sad: I can also make a pr for the docs this weekend if I'm not busy. |
So, I have a question. Are the soft-deleted entities supposed to show up in the related entities when I execute a findOne()? Is there a specific way to suppress the soft-deleted related entities? I'm looking for a way of doing this using the repository, not the queryBuilder, if possible. The BloodPressure entities have the "deleted" field correctly set by TypeORM when I call softDelete() but they still show up in the array of related entities. Can someone help me out with this? Thanks in advance! (sample code below)
|
@TheDuckman When you don't want to suppress the soft-deleted related entities in your left join, maybe you should add the condition to the JOIN's ON clause. |
But I don't see a way of setting the "ON" clause explicitly on the JoinOptions of FindOneOptions (repository findOne() options types). From what I've seen, that is only possible when using the queryBuilder. Am I missing something? If that's the only way, I'll have to use the queryBuilder but I'd really rather use the repository. |
EDITEDI am not sure if opening an issue that is why I am writting here... I just tried to make cascade working with soft-delete but is not working with child related tables. Only works with first entity... This is the parent entity() @Entity('user')
export class User {
@PrimaryGeneratedColumn({ type: 'bigint' })
id: number;
@Index({ unique: true })
@Column({ type: 'varchar', length: 20, nullable: false })
username: string;
@Column({ type: 'varchar', length: 500, nullable: true, select: false })
password: string;
@OneToMany(type => UserMetadata, meta => meta.user, { eager: true, cascade: true })
@JoinTable()
metadata: UserMetadata[];
@DeleteDateColumn({ name: 'deleted_at' })
public deletedAt: Date;
} The user metadata entity looks like that: @Entity('user_metadata')
export class UserMetadata {
@PrimaryGeneratedColumn({ type: 'bigint' })
id: number;
@Column({ type: 'varchar', length: 255, nullable: true })
key: string;
@Column({ type: 'varchar', length: 255, nullable: true })
value: string;
@Index()
@ManyToOne(type => User, (user) => user.metadata)
@JoinColumn({ name: 'user_id' })
user!: User;
@DeleteDateColumn({ name: 'deleted_at' })
public deletedAt: Date;
} This is after soft-removing user entity And this is the user_metadata table As you can see soft-deleted did not applied in cascade... I think a method like onSoftDelete should be supported? EditionThe reason why the cascade was not working was because softDelete unlike save method executes a primitive operation without cascades, relations and other operations included... for cascade should be used softRemove which receives the Entity as a parameter. In the other hand for recovering also there's the same behavior with recover && restore, restore does the restore but without the cascade, etc. The recover receives an Entity as a parameter and does the cascade for you, however in fact How do you find a softdeleted Entity to provide this as a parameter for recovering? |
@ruslanguns To recover a soft-deleted entity with all of it's relations, you should provide "withDeleted: true" as an Option when getting that specific entity:
PS: Also provide what relations you want to be fetched with your Entity. |
does this not work for ActiveRecord at the moment? |
is there anyone working on this? |
@jaequery Hi, I can make a PR for it in the next few days. |
@iWinston that would be amazing Winston, thanks |
Hi @iWinston was this ever implemented? |
See the "merged" flag on the PR.. |
@iWinston is there any limitations on specific databases for this to work? More specifically, for my use case we need this functionality in MongoDB? From my small understanding of how TypeORM works, I believe the implementation of the MongoDB driver is quite different under the hood compared to SQL databases, a cmd+f in this thread doesn't seem to mention it 🧐 |
@iWinston what are your thoughts on respecting the In his example, the YES, we could use the query builder and build our own queries everywhere throughout the app. Though if that's the case, then we don't really need the I feel like everywhere throughout the ORM, and it's internal query builders, it should respect the |
@pleerock @iWinston hi guys seems like soft deletion and |
@pleerock @iWinston
(little bit cut code fragment) |
@danielzev have you found any solution. We hit the same problem where having relationship with eager loading will load them all even the soft deleted ones. |
softDelete
andrestore
methods to query buildersoftDelete
andrestore
methods to repositorysoftRemove
andrecover
methods to repository